[Merged by Bors] - Add background file logging #2762

macladson · 2021-11-02T05:54:54Z

Issue Addressed

Closes #1996

Proposed Changes

Run a second Logger via sloggers which logs to a file in the background with:

separate debug-level for background and terminal logging
the ability to limit log size
rotation through a customizable number of log files
an option to compress old log files (.gz format)

Add the following new CLI flags:

--logfile-debug-level: The debug level of the log files
--logfile-max-size: The maximum size of each log file
--logfile-max-number: The number of old log files to store
--logfile-compress: Whether to compress old log files

By default background logging uses the debug log level and saves logfiles to:

Beacon Node: $HOME/.lighthouse/$network/beacon/logs/beacon.log
Validator Client: $HOME/.lighthouse/$network/validators/logs/validator.log

Or, when using the --datadir flag:
$datadir/beacon/logs/beacon.log and $datadir/validators/logs/validator.log

Once rotated, old logs are stored like so: beacon.log.1, beacon.log.2 etc.

Note: beacon.log.1 is always newer than beacon.log.2.

Additional Info

Currently the default value of --logfile-max-size is 200 (MB) and --logfile-max-number is 5.
This means that the maximum storage space that the logs will take up by default is 1.2GB.
(200MB x 5 from old log files + <200MB the current logfile being written to)
Happy to adjust these default values to whatever people think is appropriate.

It's also worth noting that when logging to a file, we lose our custom slog formatting. This means the logfile logs look like this:

Oct 27 16:02:50.305 INFO Lighthouse started, version: Lighthouse/v2.0.1-8edd9d4+, module: lighthouse:413
Oct 27 16:02:50.305 INFO Configured for network, name: prater, module: lighthouse:414

paulhauner

This looks good! I noticed the logs have a slightly different format, probably because they're skipping our decorator. Here's an example:

File:

Nov 22 16:34:54.000 INFO Synced, slot: 1754274, block: 0xe04c…dbf8, epoch: 54821, finalized_epoch: 54819, finalized_root: 0x502c…f45c, peers: 49, service: slot_notifier, module: client::notifier:266

Terminal:

Nov 22 16:34:54.000 INFO Synced                                  slot: 1754274, block: 0xe04c…dbf8, epoch: 54821, finalized_epoch: 54819, finalized_root: 0x502c…f45c, peers: 49, service: slot_notifier

There's a difference in spacing the also the module/service keys. For consistency, is it easy to run them both through the same decorator to get the same formatting?

I also noticed that JSON formatting is missing from the logfiles. Is that difficult to obtain as well?

lighthouse/src/main.rs

macladson · 2021-11-22T06:10:18Z

For consistency, is it easy to run them both through the same decorator to get the same formatting?

My understanding is that sloggers builds its loggers with pre-defined decorators:
https://github.com/sile/sloggers/blob/master/src/file.rs#L156-L178

It may be possible to add a Custom variant to the enum where we could define our own decorator (I could look into implementing this myself if you'd like).

I also noticed that JSON formatting is missing from the logfiles. Is that difficult to obtain as well?

In that same piece of code, JSON can be enabled as a logging format in the form of a feature. So this should be possible. I'll get to adding this.

michaelsproul

LGTM!

The only thing I spotted was in the issue description you say:

Or, when using the --datadir flag:
$datadir/$network/beacon/logs/beacon.log and $datadir/$network/validators/logs/validator.log

but the actual logging path is $datadir/beacon/logs/beacon.log, which I think is better anyway.

macladson · 2021-11-24T06:53:52Z

The only thing I spotted was in the issue description you say:
...

Oh yeah, I got mixed up regarding how the --datadir flag works. It doesn't append the network to the path. My mistake 👍

paulhauner · 2021-11-30T03:25:16Z

bors r+

## Issue Addressed Closes #1996 ## Proposed Changes Run a second `Logger` via `sloggers` which logs to a file in the background with: - separate `debug-level` for background and terminal logging - the ability to limit log size - rotation through a customizable number of log files - an option to compress old log files (`.gz` format) Add the following new CLI flags: - `--logfile-debug-level`: The debug level of the log files - `--logfile-max-size`: The maximum size of each log file - `--logfile-max-number`: The number of old log files to store - `--logfile-compress`: Whether to compress old log files By default background logging uses the `debug` log level and saves logfiles to: - Beacon Node: `$HOME/.lighthouse/$network/beacon/logs/beacon.log` - Validator Client: `$HOME/.lighthouse/$network/validators/logs/validator.log` Or, when using the `--datadir` flag: `$datadir/beacon/logs/beacon.log` and `$datadir/validators/logs/validator.log` Once rotated, old logs are stored like so: `beacon.log.1`, `beacon.log.2` etc. > Note: `beacon.log.1` is always newer than `beacon.log.2`. ## Additional Info Currently the default value of `--logfile-max-size` is 200 (MB) and `--logfile-max-number` is 5. This means that the maximum storage space that the logs will take up by default is 1.2GB. (200MB x 5 from old log files + <200MB the current logfile being written to) Happy to adjust these default values to whatever people think is appropriate. It's also worth noting that when logging to a file, we lose our custom `slog` formatting. This means the logfile logs look like this: ``` Oct 27 16:02:50.305 INFO Lighthouse started, version: Lighthouse/v2.0.1-8edd9d4+, module: lighthouse:413 Oct 27 16:02:50.305 INFO Configured for network, name: prater, module: lighthouse:414 ```

bors · 2021-11-30T04:56:03Z

Pull request successfully merged into unstable.

Build succeeded:

macladson added the blocked label Nov 2, 2021

macladson mentioned this pull request Nov 2, 2021

Logging should be enabled by default #1996

Closed

macladson added 4 commits November 16, 2021 09:18

Add background file logging

267292b

Fix udeps

c7c1872

Use new sloggers version

f55c09c

Adjust default logfile-max-size

4cefaf8

macladson force-pushed the default-logging branch from af9a78f to 4cefaf8 Compare November 15, 2021 22:55

macladson added ready-for-review The code is ready for review and removed blocked labels Nov 15, 2021

paulhauner reviewed Nov 22, 2021

View reviewed changes

lighthouse/src/main.rs Outdated Show resolved Hide resolved

paulhauner added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Nov 22, 2021

macladson added 2 commits November 23, 2021 12:30

Use MiB instead of MB

9224b4f

Allow JSON formatted file logs

6becfe8

macladson added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Nov 23, 2021

michaelsproul approved these changes Nov 24, 2021

View reviewed changes

paulhauner approved these changes Nov 30, 2021

View reviewed changes

paulhauner added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Nov 30, 2021

bors bot changed the title ~~Add background file logging~~ [Merged by Bors] - Add background file logging Nov 30, 2021

bors bot closed this Nov 30, 2021

macladson deleted the default-logging branch November 30, 2021 05:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Add background file logging #2762

[Merged by Bors] - Add background file logging #2762

macladson commented Nov 2, 2021 •

edited

Loading

paulhauner left a comment •

edited

Loading

macladson commented Nov 22, 2021

michaelsproul left a comment

macladson commented Nov 24, 2021

paulhauner commented Nov 30, 2021

bors bot commented Nov 30, 2021

[Merged by Bors] - Add background file logging #2762

[Merged by Bors] - Add background file logging #2762

Conversation

macladson commented Nov 2, 2021 • edited Loading

Issue Addressed

Proposed Changes

Additional Info

paulhauner left a comment • edited Loading

Choose a reason for hiding this comment

macladson commented Nov 22, 2021

michaelsproul left a comment

Choose a reason for hiding this comment

macladson commented Nov 24, 2021

paulhauner commented Nov 30, 2021

bors bot commented Nov 30, 2021

macladson commented Nov 2, 2021 •

edited

Loading

paulhauner left a comment •

edited

Loading